Skip to content

chore(action-button): implement accessibility features#6348

Open
cdransf wants to merge 4 commits into
cdransf/s2-action-button-migrationfrom
cdransf/s2-action-button-migration-a11y
Open

chore(action-button): implement accessibility features#6348
cdransf wants to merge 4 commits into
cdransf/s2-action-button-migrationfrom
cdransf/s2-action-button-migration-a11y

Conversation

@cdransf
Copy link
Copy Markdown
Member

@cdransf cdransf commented May 28, 2026

Description

Implements Phases 2–4 of the 1st-gen → 2nd-gen swc-action-button migration:

  • Phase 2 (setup): Directory scaffold, file stubs, and package exports for the new swc-action-button component in both 2nd-gen/packages/core (types) and 2nd-gen/packages/swc (component, stories, tests).
  • Phase 3 (API): Full TypeScript implementation of ActionButton extending ButtonBase, including the xsxl size scale, quiet, static-color, and the no-default-attribute size behavior via a _size=null sentinel.
  • Phase 4 (accessibility): ARIA passthrough for aria-haspopup / aria-expanded (forwarded to the inner <button>, then stripped from the host), a pending-icon element for Phase 5 CSS targeting, and aligned JSDoc across all public members.

Also adds @deprecated JSDoc and window.__swc.warn() runtime warnings to the 1st-gen sp-action-button for emphasized, holdAffordance, selected, and toggles — signaling the migration path to consumers.

Motivation and context

swc-action-button is a high-frequency component used in toolbars, action groups, and icon-first chrome. The 2nd-gen version removes toggle semantics (toggles, selected, aria-pressed), the link API (href), and the consumer-controlled role override — all of which were identified in the migration analysis as footguns or anti-patterns. Toggle / selection UX moves to the forthcoming swc-toggle-button / swc-toggle-button-group.

Related issue(s)

  • SWC-2044

Screenshots (if appropriate)

N/A — no visual output yet (CSS is a stub; Phase 5 adds styling).

Author's checklist

  • I have read the CONTRIBUTING and PULL_REQUESTS documents.
  • I have reviewed the Accessibility Practices for this feature, see: Aria Practices
  • I have added automated tests to cover my changes.
  • I have included a well-written changeset if my change needs to be published.
  • I have included updated documentation if my change required it.

Note on tests: Phase 6 adds play-function assertions and ARIA snapshot tests. The test files created here (action-button.test.ts, action-button.a11y.spec.ts) are stubs intentionally left empty until the styling phase is complete and there is a renderable component to assert against.

Reviewer's checklist

  • Includes a Github Issue with appropriate flag or Jira ticket number without a link
  • Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
  • Automated tests cover all use cases and follow best practices for writing
  • Validated on all supported browsers
  • All VRTs are approved before the author can update Golden Hash

Manual review test cases

  • 1st-gen deprecation warnings fire in dev mode

    1. Open any 1st-gen Storybook story for sp-action-button
    2. Open the browser console
    3. Set emphasized, holdAffordance, selected, or toggles on the element
    4. Expect: a window.__swc.warn() deprecation warning appears for each
  • 2nd-gen size attribute not reflected by default

    1. Open the 2nd-gen Storybook Playground for swc-action-button
    2. Inspect the host element in DevTools
    3. Expect: no size attribute on the host until one is explicitly set; button.size JS property returns 'm'
  • ARIA passthrough: aria-haspopup / aria-expanded forwarded and stripped

    1. In a 2nd-gen Storybook story, set aria-haspopup="true" and aria-expanded="false" on a <swc-action-button>
    2. Inspect the host element in DevTools
    3. Expect: host carries neither aria-haspopup nor aria-expanded
    4. Inspect shadowRoot.querySelector('button') in the console
    5. Expect: the inner <button> carries both aria-haspopup="true" and aria-expanded="false"

Device review

  • Did it pass in Desktop?
  • Did it pass in (emulated) Mobile?
  • Did it pass in (emulated) iPad?

Accessibility testing checklist

Required: Complete each applicable item and document your testing steps.

  • Keyboard (required — document steps below)

    1. Open the 2nd-gen Storybook Playground for swc-action-button
    2. Tab to the component
    3. Expect: focus ring visible on the inner <button>, not the host; shadowRoot.activeElement returns the inner <button>
    4. Press Enter and Space
    5. Expect: click event fires and bubbles to the host
    6. Set pending attribute; tab back to the button
    7. Expect: button is still in tab order; Enter / Space do not fire click
    8. Set disabled attribute; tab through the page
    9. Expect: button is skipped entirely
  • Screen reader (required — document steps below)

    1. Open the 2nd-gen Storybook Playground for swc-action-button with VoiceOver (macOS) or NVDA (Windows)
    2. Tab to the default <swc-action-button>Edit</swc-action-button>
    3. Expect: announces "Edit, button"
    4. Tab to an icon-only button with accessible-label="Format"
    5. Expect: announces "Format, button"; host element is not announced separately
    6. Set aria-haspopup="true" and aria-expanded="false" on the button
    7. Expect: announces "Edit, button, has popup" (or platform equivalent)
    8. Set pending attribute
    9. Expect: announces something like "Edit, busy, button" (aria-disabled state)

@cdransf cdransf requested a review from a team as a code owner May 28, 2026 18:30
@cdransf cdransf self-assigned this May 28, 2026
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 28, 2026

⚠️ No Changeset found

Latest commit: 0952e48

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@cdransf cdransf added Component:Action button Spectrum 2 Issues related to Spectrum 2 Status:WIP PR is a work in progress or draft labels May 28, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 28, 2026

📚 Branch Preview Links

🔍 First Generation Visual Regression Test Results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

Deployed to Azure Blob Storage: pr-6348

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

@cdransf cdransf mentioned this pull request May 28, 2026
13 tasks
@cdransf cdransf force-pushed the cdransf/s2-action-button-migration-a11y branch 2 times, most recently from 1df39ae to 2d0ee29 Compare May 29, 2026 15:36
@cdransf cdransf added Status:Ready for review PR ready for review or re-review. and removed Status:WIP PR is a work in progress or draft labels May 29, 2026
@cdransf cdransf force-pushed the cdransf/s2-action-button-migration-a11y branch 4 times, most recently from 5e65e8d to 20eab7c Compare June 2, 2026 16:23
* `hold-affordance` attribute and `longpress` event will be removed in a
* future release.
*/
@property({ type: Boolean, reflect: true, attribute: 'hold-affordance' })
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not deprecated technically. its just not included in scope of MVP but should be addressed sooner than later.

For all deprecation notices: Also we might want to have a chat in a sync about the deprecation language because these are a bit confusing to me since there wont be a future release its removed and we are referencing gen2 components that dont exist yet

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right on! I updated it but can revise as needed. ✨

@cdransf cdransf requested a review from caseyisonit June 2, 2026 19:44
Comment on lines +66 to +68
const classValidSizes = (
this.constructor as unknown as SizedElementConstructor
).VALID_SIZES;
Copy link
Copy Markdown
Contributor

@caseyisonit caseyisonit Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you share why this is needed? validSizes is passed in as an arg and this is disconnecting it from usage.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, it's more or less because ButtonBase provides BUTTON_VALID_SIZES which doesn't have xs, preventing that size from being set. If we don't need xs we could get rid of this. ✨

Comment thread 2nd-gen/packages/swc/components/action-button/ActionButton.ts Outdated
@cdransf cdransf requested a review from caseyisonit June 2, 2026 23:29
@rise-erpelding rise-erpelding self-assigned this Jun 2, 2026
@cdransf cdransf force-pushed the cdransf/s2-action-button-migration-a11y branch from 04da994 to 75049a7 Compare June 3, 2026 15:29
Copy link
Copy Markdown
Collaborator

@rise-erpelding rise-erpelding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The base branch needs to be changed from main to the feature branch.

Otherwise, though, this looks pretty good - are a11y tests going to be implemented in the testing phase?

@cdransf cdransf requested a review from rise-erpelding June 4, 2026 15:23
@cdransf cdransf changed the base branch from main to cdransf/s2-action-button-migration June 4, 2026 20:07
@cdransf
Copy link
Copy Markdown
Member Author

cdransf commented Jun 4, 2026

The base branch needs to be changed from main to the feature branch.

Otherwise, though, this looks pretty good - are a11y tests going to be implemented in the testing phase?

Doh! Fixed that. I'll fit the a11y tests in with testing — that'll have these changes in it and we can consolidate everything in the upstream testing branch. ✨

@cdransf cdransf force-pushed the cdransf/s2-action-button-migration branch from b028dc5 to bfb1116 Compare June 4, 2026 20:12
@cdransf cdransf force-pushed the cdransf/s2-action-button-migration-a11y branch from 5a8c394 to b6205f6 Compare June 4, 2026 20:12
@cdransf cdransf force-pushed the cdransf/s2-action-button-migration branch from bfb1116 to cd83f2c Compare June 5, 2026 15:47
@cdransf cdransf force-pushed the cdransf/s2-action-button-migration-a11y branch from b6205f6 to 0952e48 Compare June 5, 2026 15:48
Copy link
Copy Markdown
Collaborator

@rise-erpelding rise-erpelding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

@cdransf cdransf requested a review from nikkimk June 5, 2026 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component:Action button Spectrum 2 Issues related to Spectrum 2 Status:Ready for review PR ready for review or re-review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants